Skip to content

Fix Linting #4123

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Fix Linting #4123

wants to merge 6 commits into from

Conversation

@holgerd77
Copy link
Member

@holgerd77 holgerd77 commented Sep 2, 2025

Linting is locally broken if e.g. used in the context of the ESLint extension in VSC due to the around-the-corner config inclusion in the root folder eslint.config.mjs file, this is the output from the linting console log in VSC:

2025-09-02 09:48:58.180 [info] ESLint server is starting.
2025-09-02 09:48:59.319 [info] ESLint server running in node v22.17.0
2025-09-02 09:48:59.322 [info] ESLint server is running.
2025-09-02 09:49:00.110 [info] ESLint library loaded from: /EthereumJS/ethereumjs-monorepo/node_modules/eslint/lib/api.js
2025-09-02 09:49:07.685 [warning] Linting file file:///EthereumJS/ethereumjs-monorepo/packages/evm/src/precompiles/05-modexp.ts took 5650ms. Please check the ESLint rules for performance issues.
2025-09-02 09:49:17.634 [error] Calculating config file for file:///EthereumJS/ethereumjs-monorepo/eslint.config.mjs) failed.
Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/config/eslint.config.mjs' imported from /ethereumjs-monorepo/eslint.config.mjs
    at finalizeResolution (node:internal/modules/esm/resolve:279:11)
    at moduleResolve (node:internal/modules/esm/resolve:870:10)
    at defaultResolve (node:internal/modules/esm/resolve:994:11)
    at ModuleLoader.defaultResolve (node:internal/modules/esm/loader:780:12)
    at #cachedDefaultResolve (node:internal/modules/esm/loader:704:25)
    at ModuleLoader.resolve (node:internal/modules/esm/loader:687:38)
    at ModuleLoader.getModuleJobForImport (node:internal/modules/esm/loader:305:38)
    at ModuleJob._link (node:internal/modules/esm/module_job:175:49)

This prevents e.g. auto-linting on save. Removing this config "forwarding" from the root eslint.config.mts. fixes this:

grafik

First PR run is to test if this change has side effects on CI. Otherwise I would merge.

@codecov
Copy link

codecov bot commented Sep 2, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.32%. Comparing base (6011468) to head (d3ecf57).
⚠️ Report is 6 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

Flag Coverage Δ
block 84.33% <ø> (ø)
blockchain 89.32% <ø> (ø)
common 97.51% <ø> (ø)
evm 72.77% <100.00%> (ø)
mpt 90.05% <ø> (ø)
statemanager 69.06% <ø> (ø)
static 99.77% <ø> (ø)
tx 89.89% <ø> (ø)
util 88.71% <ø> (ø)
vm 55.31% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@holgerd77
Copy link
Member Author

holgerd77 commented Sep 2, 2025

Ugh. Impressively enough I am not getting the ignore rule from eslint.config.mts to work on the execution-spec-tests folder. 🤯

So npm run lint from root fails atm. I crawled a bit through this thread, a bit frustrating though, would need some fresh energy for this.

For me this issue is important though - I feel direct linting is such a relief! - so I will keep on it. Happy for some insights though on this as well!

@gabrocheleau
Copy link
Contributor

gabrocheleau commented Sep 6, 2025

For the CI, could it be that the issue comes from the formatting (i.e. Biome) rather than the actual linting from eslint? I'm getting eslint to work locally, but biome throws some formatting error related to these files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

package: monorepo PR state: needs changes type: CI type: test no browser type: test skip most VM

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@holgerd77 @gabrocheleau